Fix list_nodes in the Azure arm driver #1850
Conversation
|
Thanks for the contribution. It would be great if you could also add a unit test for this change. |
Signed-off-by: Matthias Gatto <[email protected]>
Signed-off-by: Matthias Gatto <[email protected]>
Cloud Files has no concept of an "internal" CDN so the endpoint type must be hard-coded to "external". Sample Cloud Files service catalog listing: https://docs.rackspace.com/docs/cloud-files/v1/getting-started/authenticate#cloud-files-auth-response
|
@Kami sorry for the long delay, but I've added some unit tests now. Could you please have another look at the PR? |
and reformat the code.
…d..d499194be d499194be README: prep 1.0.8 cf52d21d8 Remove pin on requests (fixes apache#41) (apache#42) 3ac8fed01 README: prep 1.0.7 (apache#40) f7e969538 requirements: constrain requests below 2.30 (apache#39) 28aa5e1be New issue templates (apache#37) 1abec09c8 action: replace `internal-be-careful-debug` (apache#36) 75edeacda README: prep 1.0.6 (apache#34) 666b1b883 Check that output file exists before opening (apache#33) git-subtree-dir: .github/actions/gh-action-pip-audit git-subtree-split: d499194be74aeb3bc7dbed3a224a87e1831132c7
just use version which is installed in the end user environment).
… outscale-mgo-trunk
…llamasoft-patch-1
|
@jan-mue Thanks for adding a test case. I will review it shortly and if everything looks fine, merge changes into trunk. |
| ] | ||
| params = {"api-version": VM_API_VERSION} | ||
| nodes = [] | ||
| while True: |
There was a problem hiding this comment.
I think this code is far for now, but to be on the safe side and avoid possible infinite loop, I think it would be good to add additional guard here - e.g. bail out if we are still stuck in this loop for more than X seconds.
…cloud into jan-mue-fix-azure-list-nodes
|
@jan-mue Can you please also sign an ICLA when you get a chance (https://libcloud.readthedocs.io/en/latest/development.html#contributing-bigger-changes)? Thanks. |
returns invalid response or similar. Also add a test case for it.
|
I pushed a change so we don't end up in an infinite loop in case server returns a bad response, there is a bug in the code or similar - c49ecd0. I picked that 60 seconds limit arbitrarily, but if needed or wanted, it can be changed (e.g. increased) - @jan-mue Can you please advise if 60 seconds is enough or the limit should be higher? I imagine we may want a higher limit since depending on the method arguments, each For now, in case deadline is reached we simply return, but we may also want to throw or at least log a warning in such scenario. |
|
EDIT: I went ahead and bumped that limit to 300 seconds for now. This way I can wrap up and merge this PR. And then based on your future feedback @jan-mue, we can adjust this value. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #1850 +/- ##
==========================================
+ Coverage 83.11% 83.12% +0.01%
==========================================
Files 353 353
Lines 81269 81317 +48
Branches 8587 8592 +5
==========================================
+ Hits 67541 67588 +47
- Misses 10922 10924 +2
+ Partials 2806 2805 -1
|
Fix the list_nodes function in the Azure ARM driver
Description
The Azure API returns an attribute
nextLinkfor pagination that is currently not used in the Azure ARM driver.Documentation: https://learn.microsoft.com/en-us/rest/api/compute/virtual-machines/list-all?tabs=HTTP
This PR fixes the issue where not all VMs are returned for subscriptions with a lot of VMs (#1824)
Status
ready for review
Checklist (tick everything that applies)